Skip to content

Conversation

@arjxn-py
Copy link
Member

@arjxn-py arjxn-py commented Aug 19, 2025

Description

Simple Symbol:

75135

Graduated:

19323

Categorised:

63912

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

📚 Documentation preview: https://jupytergis--880.org.readthedocs.build/en/880/
💡 JupyterLite preview: https://jupytergis--880.org.readthedocs.build/en/880/lite

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch arjxn-py/jupytergis/legends

@github-actions
Copy link
Contributor

github-actions bot commented Aug 19, 2025

Integration tests report: appsharing.space

@arjxn-py arjxn-py added the enhancement New feature or request label Aug 19, 2025
} finally {
setIsLoading(false);
}
}, [layerId, model]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the dependencies include the whole model, or just the piece of the model we care about listening to? Maybe we're firing this hook too much, or not at all because the reference isn't changing but deeply nested values are changing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now using model awareness to update legends - which is why I need model.

@arjxn-py arjxn-py changed the title [wip]: legends Legends for Vector Layers Aug 25, 2025
@arjxn-py arjxn-py marked this pull request as ready for review August 25, 2025 11:43
@martinRenou
Copy link
Member

I love it

Screenshot From 2025-08-26 09-55-40

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I'll leave some time to @mfisher87 for having another look if he wants

@mfisher87
Copy link
Member

mfisher87 commented Aug 26, 2025

Quick drive-by before another meeting, I'll look more at the code later:

I think categorized and graduated are reversed. I'd like to see a gradient for graduated symbology, and swatches for categorized, since one color applies to one value (or, ideally, a range of values #902) with categorized symbology.

And for graduated, is it correct to say that, e.g. for earthquakes, 3.33-4.13 are exactly one color? If I remember correctly, we use linear interpolation between the classes, and if not, we should!

The problem for categorized is that the number of swatches can get huge with many unique values, but that's not an issue with the legend, that's an issue with the user's choice to use catetegorized symbology and/or our implementation of it.

  • Choice: The user has MANY unique values and chooses categorized symbology when they really want graduated
  • Implementation: Our categorized implementation doesn't allow the user to manually group values, e.g. 0-15 as category A, 15-20 as category B, and 20-100 as category C.

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Let's get this in and open an issue for the heatmap case

@martinRenou martinRenou merged commit d163122 into geojupyter:main Aug 27, 2025
14 checks passed
@mfisher87
Copy link
Member

Beautiful :)

mfisher87 pushed a commit to mapninja/jupytergis-docwork that referenced this pull request Aug 28, 2025
* legends wip

* lint

* categorised working

* style fix

* don't fire too often

* do something

* rework

* style fix

* horizontal colorbar

* not needed

* not needed

* lint

* simplify hook

* property label

* conditionally render legend

* lint

* swamps for categorised and gradient for graduated

* max height and overflow for categorised
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants